Spike: Microsoft Teams Adapter (Bot Framework) (ADR 0007) - #4
Draft
ThomasK33 wants to merge 3 commits into
Draft
Conversation
Implements the ADR 0007 Teams adapter as a code spike under adapters/msteams, behind the existing chat.Adapter interface with the runtime and core types unchanged. This is a SPIKE: exercised end to end against fake Bot Framework servers, but NOT yet validated against a real Azure Bot resource / Teams tenant. The ADR Open Questions remain the live-validation checklist. What it does: - Inbound: validates the per-request Bot Connector JWT against the OpenID/JWKS metadata (Bearer, RS256-only, kid, signature, iss, aud==AppID, exp/nbf with 5-min skew, serviceurl claim bound to Activity.serviceUrl), with a JWKS cache (>=24h, refresh on kid miss/rotation) and a strict fail-closed channel endorsement check. No switch disables validation. - Outbound: client_credentials token minted and cached in process memory, lazily refreshed; replies/proactive posts go out as separate authenticated Connector "send to conversation" REST calls. Runtime State stores no credentials. - Opaque Thread ID serializes the minimal conversationReference (serviceUrl, conversation id, tenant id, bot id, channel id) so posting survives restarts; ValidateThreadID round-trips it for Thread Handle reconstruction. - Normalizes message Activities to Event/Message/Actor: bot-mention stripping, Mentioned from entities (never text matching), tenant-safe self-filtering, personal-scope DirectMessage, full Activity preserved via the Platform Escape Hatch. Outbound rate-limit retry (ADR 0005) reuses internal/ratelimit and is surfaced through the Observer (ADR 0010). Key deviation (spike finding, resolves Open Question 9): JWT/JWKS validation is standard-library only, so the zero-dependency core gains no JWT library; msbotbuilder-go is not adopted and golang-jwt is unnecessary. Deferred per ADR: Adaptive Card native content / interaction + command invokes (0004/0003), multi-tenant install (0006), Graph message history (0009), ephemeral. Tested: inbound-auth matrix incl. endorsement + cache/rotation, normalization, self-filter, mention detection, Thread ID round-trip, token mint/cache/refresh, Connector posting (markdown/plain) + proactive error mapping + 429 retry, and a full chat.New runtime integration (OnNewMention -> Connector reply). go build/vet and the root module test suite are green; no new module dependencies. Change-Id: I64a892e6d4056c9b08f527ce33b7ff0ecbe2abb7 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas Kosiewski <tk@coder.com>
Max-effort code review (9 finder angles + verify + sweep). Applied fixes; refuted/out-of-scope items noted in the PR. Security / correctness: - Endorsement bypass: checkEndorsement was fed the Activity-body channelId and short-circuited on "", so an attacker holding a token whose signing key did not endorse msteams could skip the gate by omitting channelId. Now bound to the adapter's own channel constant, never the body. (+test) - JWT with no exp was accepted as never-expiring; now exp is mandatory. (+test) - Cold-cache JWKS/metadata outage returned 403 (which the Connector treats as permanent, dropping a valid Activity); a transient key-fetch failure now returns a retryable 503 via a typed errKeysUnavailable sentinel. (+test) - RSA public exponent was built via big.Int.Int64() with no bound, risking silent truncation (32-bit int); now range-checked. - PostMessage trusted ThreadRef.Raw via an unchecked type assertion; now always decodes the authoritative opaque Thread ID, so a mismatched Raw cannot misroute a reply. - Webhook no longer leaks internal error strings on a normalize failure (generic 400 + structured log), matching the Slack adapter. Correctness/clarity: - stripBotMention strips only the first mention occurrence (was ReplaceAll), so a token the user legitimately repeats is preserved; mention scan computed once. - Deduplicated the DirectMessage rule via conversationReference.direct(). - Corrected BotActor()/normalizeActivity comments: the adapter-internal self-drop is authoritative (the runtime's tenant-scoped isSelfActor cannot fire for a cross-tenant Teams bot) and depends on BotID being correct (spike-required). - Removed a dead Adapter.appID field. Refuted (no change): the serviceUrl SSRF/token-exfil concern — the serviceurl claim is verified against the signed token before use, so it cannot be steered to an arbitrary host. Noted out of scope: factoring the retry loop / noopObserver / firstNonEmpty / thread-id codec into shared helpers (touches Slack/Linear), and single-flight on the JWKS/token caches. Change-Id: I3be83810514260873154700135d1783351227105 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas Kosiewski <tk@coder.com>
The spike's unexported helpers carried multi-line doc essays where the sibling Slack/Linear adapters leave theirs comment-free. Trimmed those (activity.go normalize/actor/mention/strip, the conversationReference and authValidator type docs, checkEndorsement, the Webhook and PostMessage docs) to 1-3 lines, keeping the non-obvious security/design rationale and the terse spike-required markers. Inlined connRefForThread (a one-line wrapper post-review). Comment-only changes plus the inline; no behavior change, tests unchanged and green. Change-Id: Icfb9c549a5b70b460e7c533ca7ab0462386ac572 Signed-off-by: Thomas Kosiewski <tk@coder.com>
Member
Author
|
Maintainer triage verdict: stays draft — blocked on live-tenant validation. Tracking: #6. Assessment: this is a strong spike. Self-contained under What it can't do from a mock: 9 of ADR 0007's 10 open questions (ack/turn semantics, production key endorsements, markdown fidelity, Decision:
Generated with |
This was referenced Aug 27, 2026
ThomasK33
added a commit
that referenced
this pull request
Aug 27, 2026
…o new deps) (#14) * docs(adr): record ADR 0007 spike finding — Q9 resolved (stdlib JWT, no new deps) The Teams adapter spike (PR #4) resolved Open Question 9: msbotbuilder-go is rejected as unmaintained, and inbound JWT/JWKS validation was implemented with the standard library only (crypto/rsa), adding zero new dependencies. Recorded on main so the finding survives the spike branch. Remaining open questions still require live-tenant validation (issue #6); the ADR status stays Proposed until then. --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `anthropic:claude-fable-5` • Thinking: `xhigh`_ * docs(adr): address codex review — align Decision/PRD with stdlib choice, scope Q9, add .scratch tracker issue - Decision now records the stdlib-only JWT/JWKS choice (no contradictory 'use golang-jwt/jwx' instruction left in ADR or PRD). - Spike Findings scoped: Q9's SDK-adoption decision is resolved; its live-token verification steps are superseded and carry over into the live-validation checklist. - Live validation now tracked in .scratch/teams-adapter/issues/01 per docs/agents/issue-tracker.md, with GitHub issue #6 as public mirror. --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `anthropic:claude-fable-5` • Thinking: `xhigh`_ * docs(prd): update Teams implementation gate — live-token stdlib validation, not msbotbuilder-go evaluation The PRD's final implementation gate still required a hands-on production-readiness evaluation of msbotbuilder-go, which the ADR 0007 spike already performed and rejected. The gate now requires live-token validation of the stdlib JWT/JWKS validator instead. --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `anthropic:claude-fable-5` • Thinking: `xhigh`_
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is
A code spike of the Microsoft Teams (Bot Framework) adapter from ADR 0007, under
adapters/msteams, behind the existingchat.Adapterinterface. The runtime and core types are unchanged.Important
This is a spike, opened as a draft for a human to validate against a real Azure Bot resource + Teams tenant. It is exercised end to end against fake Bot Framework servers, but no live tenant has touched it. The ADR's Open Questions are the live-validation checklist (bottom of this PR). Do not merge as production-ready until those are confirmed.
What it implements
auth.go) — per-request Bot Connector JWT validated against the OpenID/JWKS metadata. Every mandatory check is enforced with no switch to disable: Bearer scheme, RS256-only (rejects alg confusion),kid, signature,iss == https://api.botframework.com,aud == App ID, exp/nbf with 5-minute skew, and theserviceurlclaim bound toActivity.serviceUrl. JWKS cached (≥24h) and refreshed onkidmiss / rotation. Strict, fail-closed channel-endorsement check.token.go) —client_credentialstoken minted and cached in process memory, refreshed lazily before expiry. Runtime State stores no credentials (Linear precedent).connector.go) —Thread.Post→ separate authenticated Connector "send to conversation" REST call; Plain Text →textFormat=plain, Portable Markdown →textFormat=markdown;SentMessage.IDfromResourceResponse.id. Proactive 403s map to explicitErrBotNotInstalled/ErrMessageWritesBlocked.threadid.go) — opaque, versioned serialization of the minimalconversationReference(serviceUrl, conversation id, tenant id, bot id, channel id) so posting survives restarts;ValidateThreadIDround-trips it.activity.go) — messageActivity→ Event/Message/Actor: bot-mention stripped from text,Mentionedfromentities[](never substring matching), tenant-safe self-filtering, personal-scopeDirectMessage, fullActivitypreserved via the Platform Escape Hatch.retry.go) — reuses the sharedinternal/ratelimitmechanics (ADR 0005), surfaced via the Observer (ADR 0010); never sleeps past the caller deadline.Key design decision (deviation — resolves Open Question 9)
Inbound JWT/JWKS validation is standard-library only (
crypto/rsaover a public key rebuilt from the JWKn/e). The root module is deliberately zero-dependency (Slack/Linear are pure-stdlib direct-HTTP) andadapters/msteamslives in it, so pulling a JWT lib would pollute the core's dependency graph.go.modis unchanged; there is nogo.sum. This deviates from the ADR's "use a maintainedgolang-jwt" note — the spike foundgolang-jwt/jwxunnecessary andmsbotbuilder-gonot worth adopting. Worth a careful review ofauth.gogiven it's security-critical and hand-rolled.Deferred per ADR (not in this slice)
Adaptive Card native content + card-action interactions / command invokes (0004/0003), multi-tenant install (0006), Graph-based message history (0009), and an
EphemeralPoster(no clean Teams equivalent).Tests (
go test ./...green on the root module; no Docker needed)Inbound-auth matrix (valid + 11 rejection cases + skew acceptance), endorsement present/absent, JWKS cache + rotation, "validation always on", normalization (channel/DM/mention/foreign-mention/actor fallback), self-message + non-message ignored, Thread ID round-trip + rejection, token mint/cache/refresh + fail-fast on bad creds, Connector posting (markdown/plain) + proactive error mapping + 429 retry/exhaustion, and a full
chat.Newruntime integration (OnNewMention→ Connector reply with the outbound bearer token).Self-review caught and fixed one bug: the leading-
<at>mention-strip fallback could strip another user's leading mention; it is now gated on the bot actually being mentioned (test added).Live-validation checklist (ADR 0007 Open Questions — each marked
spike-requiredin code)msteamsinbound ack semantics + real turn timeout (bare 200 acks amessage?).message).msteamsis absent from the signing key; confirm this doesn't reject valid production traffic.aud/iss).textFormat=markdown.serviceUrl/conversation.idpersistence stability for proactive posting.OnNewMentiononly on an explicit bot Mention entity).Actor.IDkey — spike prefersfrom.aadObjectId, falls back tofrom.id.Activity.idstability as the dedupe key across Connector redelivery.🤖 Generated with Claude Code