Skip to content

Commit 19acaef

Browse files
feat(teams): api primitive subpath + SSRF allowlist divergence (chat@4.31 8c71411) (#158)
* feat(teams): api primitive subpath + SSRF allowlist divergence (chat@4.31 8c71411) Port the Teams Bot Connector API primitives (NEW in chat@4.31.0) as a runtime-free subpath `chat_sdk.adapters.teams.api`, mirroring the `slack/api/` layout: an injectable async `fetch` (lazy `httpx` default), plain-dict activities, and no `microsoft_teams.*` / adapter imports. Ports `resolve_teams_access_token` (client_credentials grant), `call_teams_connector_api`, post/update/delete message, typing indicator, `create_teams_conversation`, `build_teams_message_activity`, and `TeamsApiError`. SSRF / token-leak divergence: `call_teams_connector_api` gates `service_url` through `is_trusted_teams_service_url` BEFORE attaching the Bearer token, raising `ValueError` for any host outside the Bot Framework allowlist (the same `ALLOWED_SERVICE_URL_PATTERNS` the high-level adapter uses). Upstream attaches the token to any caller-supplied `serviceUrl` with no host check. Documented as an append-only Known Non-Parity row in docs/UPSTREAM_SYNC.md. Tests: 13 faithful ports of `api/index.test.ts` + a source-scan boundary test (port of `api/boundary.test.ts`) + SSRF divergence/adversarial cases, all with an injected AsyncMock fetch. The lazy `teams/__init__.py` subpath registration is deferred to the packaging PR. * feat(teams): export assert_teams_ok + TeamsContinuationContext from api primitive Restore two public exports the api primitive omitted from upstream chat@4.31.0's api/client.ts / api/messages.ts: - Port assertTeamsOk -> assert_teams_ok (no-op for 2xx; raises TeamsApiError carrying the parsed body + status otherwise), add to __all__. - Define + export TeamsContinuationContext (conversation_id + service_url required, optional addressing hints) to match upstream's exported surface. Tests: - assert_teams_ok: passes silently for 2xx/204, raises with body+status on 403. - TeamsContinuationContext: required/optional field surface. - SSRF: subdomain-suffix lookalike host (smba.uk.botframework.com.attacker.example) is rejected by the anchored allowlist before any token fetch (proves the .com/ boundary anchor, not a loose suffix match). - SSRF: empty-string access_token falls through to client-credentials and raises the credentials-required error (upstream truthiness parity); no token request issued. Allowlist unchanged. UPSTREAM_SYNC SSRF row already documents the lookalike rejection; the new exports are upstream-parity (not divergences).
1 parent b0246e9 commit 19acaef

3 files changed

Lines changed: 1206 additions & 0 deletions

File tree

docs/UPSTREAM_SYNC.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -652,6 +652,7 @@ stay explicit instead of being rediscovered in code review.
652652
| Slack `api` primitives `send_slack_response_url` URL gate (vercel/chat#548) | `send_slack_response_url` (`slack/api/__init__.py`) calls `_assert_slack_response_url(url)` before POSTing — requires an `https://*.slack.com` URL (where Slack-issued `response_url`s always live) and raises `ValueError` for anything else | Upstream `api/client.ts` `sendResponseUrl` POSTs to whatever `response_url` it is handed, with no scheme/host validation | SSRF guard. The `response_url` reaching this primitive can originate from a parsed-but-unverified interaction payload; without a gate a crafted value could redirect the POST (which carries no bearer token but does echo SDK-controlled message content and trigger an arbitrary outbound request) to an attacker host. Enforces `CLAUDE.md`'s "Validate external URLs before requests (SSRF)" rule, mirroring the high-level adapter's `rehydrate_attachment` allowlist row above. Allowlist: scheme `https`, host `slack.com` or `*.slack.com`. |
653653
| Slack `api` primitives `fetch_slack_file` host allowlist (vercel/chat#548) | `fetch_slack_file` (`slack/api/__init__.py`) gates `url` through `is_trusted_slack_file_url` before forwarding the bot token, raising `ValueError` for untrusted hosts | Upstream `api/client.ts` `fetchFile` GETs the supplied URL with `Authorization: Bearer <token>` unconditionally | Token-leak guard. `fetch_slack_file` attaches the workspace bot token; a crafted `url_private` from a parsed file object could otherwise exfiltrate that token to an arbitrary host. `is_trusted_slack_file_url` requires scheme `https` and host in `{files.slack.com, slack.com, *.slack.com, *.slack-edge.com}` — the same allowlist the high-level adapter's `rehydrate_attachment` row uses for Slack. Enforces `CLAUDE.md`'s SSRF/URL-validation rule. |
654654
| Slack `web_client_options` → slack_sdk `WebClient` kwargs (vercel/chat#8336a3e, chat@4.31) | `SlackAdapterConfig.web_client_options: dict[str, Any] \| None` is spread (gated on `is not None`, so an explicit `{}` still spreads as a no-op) into **both** WebClient construction sites — the default `AsyncWebClient` (`_get_client`) and the per-token sync `WebClient` (`_get_web_client_for_token`). The keys are **slack_sdk** `WebClient` constructor kwargs: `timeout` (int seconds), `retry_handlers` (a list of `slack_sdk.http_retry.RetryHandler`), `headers`, etc. Any nested `headers` dict is **deep-copied per client** (`_web_client_kwargs`) so cached per-token clients never share a mutable dict and the caller's input is never mutated. | Upstream `webClientOptions?: Omit<WebClientOptions, "slackApiUrl">` forwards to `@slack/web-api`'s axios-backed `WebClient`; its headline keys are `retryConfig` (a `retryPolicies.*` policy), `rejectRateLimitedCalls`, and `timeout` (ms). | No 1:1 mapping: `slack_sdk` has no `retryConfig`/`rejectRateLimitedCalls` (retry behavior is configured via `retry_handlers`) and its `timeout` is seconds, not ms. So the option bag maps to slack_sdk `WebClient` kwargs rather than axios options. Same intent (tune the underlying HTTP client the adapter doesn't otherwise expose) and same per-client header isolation. Documented inline in `slack/types.py` (`web_client_options` docstring) and `slack/adapter.py` (`_web_client_kwargs`). Regression coverage: `tests/test_adapter_api_url_config.py::TestSlackWebClientOptions`. |
655+
| Teams `api` primitives `call_teams_connector_api` serviceUrl host allowlist (vercel/chat#8c71411, chat@4.31) | `call_teams_connector_api` (`teams/api/__init__.py`) gates `service_url` through `is_trusted_teams_service_url` **before** resolving/attaching the `Bearer` token, raising `ValueError` for any host outside the Bot Framework allowlist. Because every message/typing/conversation helper (`post_teams_message`, `update_teams_message`, `delete_teams_message`, `send_teams_typing`, `create_teams_conversation`) routes through the connector, the gate covers them all. | Upstream `api/client.ts` `callTeamsConnectorApi` builds `new URL(path, serviceUrl)` and POSTs with `authorization: Bearer ${token}` to whatever `serviceUrl` it is handed — no scheme/host validation | Token-leak guard. The connector forwards the bot's Bot Framework access token; a crafted `serviceUrl` (these primitives are SDK-free and accept caller-supplied values, e.g. from a parsed-but-unverified inbound activity's `serviceUrl`) could otherwise exfiltrate that token to an attacker host. `is_trusted_teams_service_url` requires scheme `https` and a host matching the Bot Framework allowlist — `{smba.trafficmanager.net, *.botframework.com, *.botframework.us, *.teams.microsoft.com, *.teams.microsoft.us, smba.infra.(gcc\|gov).teams.microsoft.(com\|us)}` — the same `ALLOWED_SERVICE_URL_PATTERNS` the high-level adapter's `_validate_service_url` uses. A slashless `serviceUrl` is trailing-slash-normalized before matching so the host-boundary anchor still holds (a lookalike like `…botframework.com.attacker.example` is rejected because the `.com/` boundary never appears). The gate runs before any token request, so an untrusted host triggers **zero** outbound calls. Enforces `CLAUDE.md`'s "Validate external URLs before requests (SSRF)" rule, mirroring the Slack `send_slack_response_url` / `fetch_slack_file` rows above. Regression coverage: `tests/test_teams_api_primitive.py::TestTeamsApiSsrfDivergence`. |
655656
### Platform-specific gaps
656657

657658
| Area | Python | TS | Rationale |

0 commit comments

Comments
 (0)