Skip to content

feat(msteams): opt-in canonical /api/messages Activity ingress (triton#247) - #248

Merged
jrosskopf merged 2 commits into
mainfrom
feat-activity-protocol-adapter
Sep 4, 2026
Merged

feat(msteams): opt-in canonical /api/messages Activity ingress (triton#247)#248
jrosskopf merged 2 commits into
mainfrom
feat-activity-protocol-adapter

Conversation

@jrosskopf

@jrosskopf jrosskopf commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Closes part of #247 (Activity Protocol adapter — the Microsoft Copilot/Teams ingress).

What

Generalizes triton-chat-msteams into triton's single, host-agnostic Microsoft Activity ingress (the "unify" decision from #247): the adapter now optionally serves the canonical Bot Framework path POST /api/messages in addition to the legacy /{name}/webhook. That is the endpoint an Azure Bot points at, and the path Teams, M365 Copilot Chat, WebChat and Copilot Studio channels all POST to. handle_webhook was already transport-generic (reads the Authorization header + raw body, not the route name), so one route reaches every Activity channel.

Design (post crew review)

  • Opt-in, single-claimant. inbound.canonical_path (default off). The path is fixed and axum::Router::merge panics on an overlapping route, so a second claimant would crash the process — and its REST/MCP/A2A listeners — at startup. triton-bin now refuses a second claimant with a named error (exit 2), matching the existing NFR-S-4 guards in the same function.
  • Auth unchanged / fail-closed. The canonical route reuses handle_webhook, so Bot Framework JWT verification (RS256, pinned kid, aud, serviceUrl allowlist) runs before the body is parsed. Forged bearers → 401, no dispatch.
  • Adaptive Card stays 1.4. The transpiler emits no property above AC 1.3 (input_widget uses only label/isRequired), and a host capped below the declared version renders fallbackText instead of the card — a downgrade on exactly the non-Teams surfaces this ingress serves. Bump only when a 1.5-only feature is actually emitted. (An earlier 1.4→1.5 + fallbackText change was reverted after the crew review flagged the downgrade risk and a notification-preview leak.)

Scope reshaped by the spike

The Phase-0 spike found Copilot Studio's external-connect path (A2A JSON-RPC) flattens Adaptive Cards to text; the rich-card path is Activity Protocol over an Azure Bot channel → Teams (which this adapter already does). So the issue's expectReplies / dev-None-auth items — which target the A2A-orchestrator/skill model — are cut, not built as dead code.

Tests

  • canonical_api_messages_path_dispatches — opted-in /api/messages dispatches identically to the legacy route.
  • api_messages_enforces_auth — forged JWT on /api/messages → 401, no outbound reply.
  • api_messages_absent_unless_opted_in/api/messages → 404 by default; /msteams/webhook still works.

Full suite green: 33 msteams lib tests + 29 msteams integration tests; clippy clean on triton-chat-msteams + triton-bin.

Follow-ups (not in this PR)

  • Consumer wiring: datazoo-agent-template opts in (TRITON_MSTEAMS_CANONICAL_PATH, default on) — lands with the vendor bump.
  • Pre-auth audit rate-limiting on public webhook paths (crew F3) — pre-existing behaviour on /msteams/webhook; filing separately.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

…n#247)

Generalize the Teams adapter into triton's single, host-agnostic Microsoft
Activity ingress (the "unify" decision). An Azure Bot's canonical messaging
endpoint is POST /api/messages — the path Teams, M365 Copilot Chat, WebChat
and Copilot Studio channels all use. handle_webhook is already transport-
generic (reads the Authorization header + raw body, not the route name), so
serving it there reaches every Activity channel, not just /msteams/webhook.

- Opt-in via `inbound.canonical_path` (default off), single-claimant: the
  path is fixed and axum's Router::merge panics on an overlap, so triton-bin
  refuses a second claimant with a named error (exit 2) instead of crashing
  the process — and REST/MCP/A2A with it — at startup.
- Auth is unchanged: the canonical route reuses handle_webhook, so Bot
  Framework JWT verification runs before the body is parsed (fail-closed).

Adaptive Card version stays 1.4: the transpiler emits no property above AC
1.3, and a host capped below the declared version renders fallbackText
instead of the card — a downgrade on exactly the non-Teams surfaces this
ingress serves. Bump it only when a 1.5-only feature is actually emitted.

Scope reshaped by the Phase-0 spike + crew review: the issue's expectReplies
/ dev-None-auth paths target the A2A-orchestrator/skill model the spike
found flattens cards, not the Azure-Bot channel path that actually carries
them — so they are cut, not built as dead code.

Tests: canonical path dispatches; forged JWT on /api/messages → 401 (auth
enforced); /api/messages absent (404) unless opted in while /msteams/webhook
still works.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jrosskopf
jrosskopf merged commit 3d77b2f into main Sep 4, 2026
3 checks passed
@jrosskopf
jrosskopf deleted the feat-activity-protocol-adapter branch September 4, 2026 04:06
jrosskopf added a commit that referenced this pull request Sep 5, 2026
…#261)

A public inbound path answers an unauthenticated probe with 401 and
audits it, all before any rate-limit token is consumed — the bucket is
deliberately taken only after auth so a sprayer can't burn it. A
background scanner therefore wrote one audit line per probe, and the
1024-entry ring buffer evicted every real entry within minutes: the
history an operator tails at /v1/audit was gone exactly when they needed
it. Pre-existing on /msteams/webhook; #248's canonical /api/messages made
the heavily-scanned path available.

Anonymous rejections now coalesce into one line per window per protocol,
carrying the count of what it stands for. Fixed in
`Dispatcher::record_rejection` — the ADR-6 audit pivot — so all seven
chat adapters and the HTTP trio inherit it with no adapter change.

Three properties the design turns on:

  * **Suppress the line, never the counter.** `metrics.record_dispatch` /
    `record_audit` stay unconditional, ahead of the window, so the exact
    rejection count survives at full per-tool granularity. Coalescing
    costs per-request repetition in the log and the buffer, nothing else.

  * **The first rejection still emits immediately, with its reason.**
    #219 exists because refusals were undiagnosable; a window that
    emitted at its END would reintroduce that for every operator staring
    at a 401. The count rides out on the next emission instead. Accepted
    trade: a flood that stops mid-window leaves its tail unreported in
    the log — the metric still has it.

  * **The key is `protocol`, not `(protocol, tool)`.** The obvious finer
    key is unbounded: the REST adapter audits a pre-auth rejection under
    `Path(name)` (the URL segment the caller picks) and MCP under a name
    off the JSON-RPC body, so keying on it lets `/v1/tools/<random>` mint
    one window per probe — a memory-growth DoS introduced by the very fix
    meant to blunt the scanner. `protocol` is a closed set.

An IDENTIFIED subject being refused is never coalesced: that is the
security signal, and a scanner sharing its protocol must not mask it.
The `subject == "-"` convention `record_rejection` already documents is
what separates the two.

`suppressed` is `Option<u64>`, skipped when absent, so a line that
swallowed nothing stays byte-identical for existing audit consumers; a
coalesced one also carries `status_detail: rejections_coalesced` since at
protocol granularity it names one tool but may span several routes.
Window from `TRITON_AUDIT_REJECT_WINDOW_SECS` (default 60, `0` disables),
wired in triton-bin so triton-core stays env-free; a junk value falls
back to the default rather than failing boot.

Tests, red first: seven no-mock integration tests against the spawned
binary over real TCP — coalescing, the unchanged 401 on every probe, the
/v1/audit tail, distinct attacker-chosen paths NOT multiplying lines,
identified rejections never coalesced (both halves on one protocol),
metrics staying lossless, the window reopening, and the msteams webhook
inheriting it all through the pivot. Plus four unit tests for the window.
The two load-bearing tests were verified to bite: keying on
(protocol, tool) reddens only the distinct-paths test; coalescing
identified rejections reddens only the identified test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant