fix(attachments): SSRF guards + Message-instance rehydration + truthiness fallbacks

Second review pass on PR #67 (rehydrate_attachment). The previous fixup
addressed pyrefly only — this commit resolves the remaining review
feedback.

SSRF guards (3 adapters)
- Slack, Teams, Google Chat all rebuild fetch_data closures from
  serialized fetch_metadata["url"] in rehydrate_attachment. A tampered
  URL in persisted queue state could exfiltrate the workspace
  bot/OAuth token to an attacker-controlled host. Each adapter now
  validates the URL's scheme (https only) and host against a
  platform-specific allowlist before forwarding the auth header.
  Upstream TS does not validate; this is a Python-first divergence
  documented in docs/UPSTREAM_SYNC.md.
    - Slack: files.slack.com, slack.com, *.slack.com, *.slack-edge.com
    - Teams: Microsoft-owned hosts (graph.microsoft.com,
      smba.trafficmanager.net, *.sharepoint.com, *.botframework.com,
      *.office.com, attachments.office.net, …)
    - Google Chat: chat.googleapis.com, *.googleapis.com,
      *.googleusercontent.com, *.google.com

Message-instance rehydration (P1)
- Chat._rehydrate_message used to early-return on Message inputs,
  matching upstream TS's `raw instanceof Message` shortcut. That
  shortcut is safe in upstream because its state adapters return raw
  JSON dicts from dequeue. Our RedisStateAdapter / PostgresStateAdapter
  both upgrade the dequeued dict to `Message.from_json(...)` before
  returning, so the early return would skip rehydrate_attachment for
  every persistent-backend dequeue and leave fetch_data stripped. We
  now fall through and apply the rehydrate pass on Message inputs too
  (already-hydrated attachments with fetch_data are filtered out).

Truthiness fallbacks (Port Rule #1)
- telegram, whatsapp rehydrate_attachment and types.py dual-key
  fetch_metadata lookup now use explicit `is not None` instead of
  `or`, so an empty-dict fetch_metadata is preserved.

Teams connection pooling
- _build_teams_fetch_data used httpx.AsyncClient as a throwaway
  context manager per download. Refactored to use the shared
  aiohttp session (_get_http_session) that the rest of the adapter
  already goes through.

Test hardening
- test_slack_webhook.py and test_teams_adapter.py now stub the fetch
  path with AsyncMock, await rehydrated.fetch_data(), and assert the
  URL + token that were forwarded. Previously the tests only checked
  that `fetch_data is not None` — they would have passed even if
  rehydration returned a dummy closure.
- New tests per adapter verify the SSRF guard rejects untrusted hosts
  and the allowlist accepts the intended Slack/Teams/GCP hosts.
- New regression test in test_chat_faithful.py drives a Message-
  instance dequeue through the chat queue and asserts
  rehydrate_attachment still fires.

Slack adapter connection pooling (deferred)
- _fetch_slack_file still uses httpx.AsyncClient per call. The Slack
  adapter has no pooled aiohttp helper (only slack_sdk.AsyncWebClient
  for Slack API calls), so adding one is a larger refactor left for
  a follow-up.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: patrick-chinchill

docs/UPSTREAM_SYNC.md

@@ -458,6 +458,8 @@ stay explicit instead of being rediscovered in code review.
 | Teams divider rendering | `card_to_adaptive_card` hoists `separator: True` onto the next sibling (or emits a non-empty Container for a trailing divider) | `convertDividerToElement` emits an empty `Container` with `separator: True` | Upstream shares the same bug: Microsoft Teams renders an empty Container at zero height, so the separator line is effectively invisible. Python port fixes locally (issue #45) rather than blocking on upstream. |
 | `SlackAdapter.current_token` / `current_client` | Public `@property` accessors that return the request-context-bound token and a preconfigured `AsyncWebClient` | Not exposed (`getToken()` is private on the TS `SlackAdapter`) | Python-only addition (issue #47). Downstream code that calls Slack Web APIs from inside a handler — email resolution, user profile fetches, reaction bookkeeping — otherwise depends on underscore-prefixed helpers. |
 | `ConcurrencyConfig.max_concurrent` | Enforced via `asyncio.Semaphore` in the `"concurrent"` strategy path; rejects non-integer or `<= 0` values, and rejects any non-`None` `max_concurrent` paired with a non-`"concurrent"` strategy | Accepted into the config type with docstring "Default: Infinity" but never read (3 writes, 0 reads) | Silent correctness bug upstream — consumers setting `max_concurrent=N` with `strategy="concurrent"` reasonably expect an N-way bound on in-flight handlers. We honor the documented contract via a semaphore and fail-fast on misconfiguration so it's never silent. `max_concurrent=None` stays compatible with every strategy (unbounded default). |
+| `rehydrate_attachment` URL allowlist (Slack / Teams / Google Chat) | Validates the downloaded URL's scheme + host against a per-adapter allowlist inside the fetch closure; raises `ValidationError` on untrusted hosts before forwarding bearer tokens | No validation — `fetchData` blindly GETs `fetchMetadata.url` and forwards the workspace/bot token | SSRF + token-exfil risk upstream: after the 4.26 `rehydrateAttachment` hook lands, a crafted `fetchMetadata` in persisted state can redirect auth'd downloads to an arbitrary host. Python port enforces `CLAUDE.md`'s "Validate external URLs before requests (SSRF)" rule. Allowlist: Slack = `{files.slack.com, slack.com, *.slack.com, *.slack-edge.com}`; Teams = `{smba.trafficmanager.net, graph.microsoft.com, attachments.office.net, *.botframework.com, *.graph.microsoft.com, *.sharepoint.com, *.officeapps.live.com, *.office.com, *.office365.com, *.onedrive.com, *.microsoft.com}`; Google Chat = `{chat.googleapis.com, googleapis.com, *.googleapis.com, *.googleusercontent.com, *.google.com}`. |
+| `_rehydrate_message` with `Message` input | Falls through to the `rehydrate_attachment` pass even when the dequeued entry is already a `Message` instance | Early-returns on `raw instanceof Message` before rehydration | The Python port's Redis + Postgres `dequeue()` upgrade raw JSON to `Message.from_json(...)` before returning (upstream's dequeue returns the raw JSON.parse'd dict). Upstream's `instanceof Message` shortcut therefore only fires for in-memory state, but ours would fire for persistent backends too, leaving `fetch_data` stripped forever. The rehydrate pass still skips any attachment that already has `fetch_data`, so in-memory callers pay no cost. |
 
 ### Platform-specific gaps
 

src/chat_sdk/adapters/google_chat/adapter.py

@@ -19,6 +19,7 @@
 from collections.abc import AsyncIterable, Awaitable, Callable
 from datetime import datetime, timezone
 from typing import Any, NoReturn
+from urllib.parse import urlparse
 
 from chat_sdk.adapters.google_chat.cards import card_to_google_card
 from chat_sdk.adapters.google_chat.format_converter import GoogleChatFormatConverter
@@ -2631,6 +2632,37 @@ def _create_attachment(self, att: dict[str, Any]) -> Attachment:
             fetch_metadata=fetch_meta or None,
         )
 
+    @staticmethod
+    def _is_trusted_gchat_download_url(url: str) -> bool:
+        """Gate Google Chat attachment downloads to Google-owned hosts.
+
+        After ``rehydrate_attachment`` reconstructs the fetch closure
+        from serialized ``fetch_metadata``, the URL may have been
+        tampered with in the state store.  We refuse to forward the
+        OAuth access token unless the host is a known Google-owned host.
+
+        This is a Python-first divergence: upstream Google Chat adapter
+        does not validate the URL.  See ``docs/UPSTREAM_SYNC.md`` Known
+        Non-Parity.
+        """
+        try:
+            parsed = urlparse(url)
+        except (ValueError, TypeError):
+            return False
+        if parsed.scheme != "https":
+            return False
+        host = (parsed.hostname or "").lower()
+        if not host:
+            return False
+        allowed_suffixes = (
+            ".googleapis.com",
+            ".googleusercontent.com",
+            ".google.com",
+        )
+        if host.endswith(allowed_suffixes):
+            return True
+        return host in {"chat.googleapis.com", "googleapis.com"}
+
     def _build_gchat_fetch_data(
         self,
         resource_name: str | None,
@@ -2656,8 +2688,15 @@ async def _fetch_data() -> bytes:
                         )
                     return await response.read()
 
-            # Fallback to direct URL fetch (downloadUri)
+            # Fallback to direct URL fetch (downloadUri).  Validate the
+            # host before forwarding the OAuth token — the URL may have
+            # been rebuilt from serialized metadata and tampered with.
             if url:
+                if not adapter._is_trusted_gchat_download_url(url):
+                    raise ValidationError(
+                        "gchat",
+                        f"Refusing to fetch Google Chat file from untrusted URL: {url}",
+                    )
                 token = await adapter._get_access_token()
                 session = await adapter._get_http_session()
                 async with session.get(
@@ -2682,7 +2721,7 @@ def rehydrate_attachment(self, attachment: Attachment) -> Attachment:
         ``url`` (fallback) from ``fetch_metadata``.  Returns the attachment
         unchanged when neither identifier is present.
         """
-        meta = attachment.fetch_metadata or {}
+        meta = attachment.fetch_metadata if attachment.fetch_metadata is not None else {}
         resource_name = meta.get("resourceName")
         meta_url = meta.get("url")
         url = meta_url if meta_url is not None else attachment.url

src/chat_sdk/adapters/slack/adapter.py

@@ -23,7 +23,7 @@
 from contextvars import ContextVar
 from datetime import datetime, timezone
 from typing import Any, NoReturn, cast
-from urllib.parse import parse_qs
+from urllib.parse import parse_qs, urlparse
 
 from chat_sdk.adapters.slack.cards import (
     card_to_block_kit,
@@ -1773,13 +1773,48 @@ async def fetch_data() -> bytes:
             fetch_metadata=fetch_meta or None,
         )
 
+    @staticmethod
+    def _is_trusted_slack_download_url(url: str) -> bool:
+        """Gate Slack file downloads to known Slack-owned hosts.
+
+        We refuse to forward ``Authorization: Bearer {token}`` to an
+        arbitrary URL.  After ``rehydrate_attachment`` reconstructs the
+        fetch closure from serialized ``fetch_metadata``, that URL may
+        have been tampered with in the state store — a crafted value
+        could exfiltrate the workspace bot token.
+
+        This is a Python-first divergence: upstream Slack adapter does not
+        validate the URL.  See ``docs/UPSTREAM_SYNC.md`` Known Non-Parity.
+        """
+        try:
+            parsed = urlparse(url)
+        except (ValueError, TypeError):
+            return False
+        if parsed.scheme != "https":
+            return False
+        host = (parsed.hostname or "").lower()
+        if not host:
+            return False
+        # Exact-match hosts
+        if host in {"files.slack.com", "slack.com"}:
+            return True
+        # Suffix match for Slack-owned subdomains
+        return host.endswith(".slack.com") or host.endswith(".slack-edge.com")
+
     async def _fetch_slack_file(self, url: str, token: str) -> bytes:
         """Download a file from a Slack ``url_private`` endpoint.
 
         Shared by :meth:`_create_attachment` (direct fetch closure) and
         :meth:`rehydrate_attachment` (reconstructed closure after JSON
-        roundtrip).
+        roundtrip).  Validates the host against the Slack allowlist
+        before forwarding the bot token (SSRF guard).
         """
+        if not self._is_trusted_slack_download_url(url):
+            raise ValidationError(
+                "slack",
+                f"Refusing to fetch Slack file from untrusted URL: {url}",
+            )
+
         import httpx
 
         async with httpx.AsyncClient() as http:
@@ -1801,10 +1836,14 @@ def rehydrate_attachment(self, attachment: Attachment) -> Attachment:
         ``attachment.fetch_metadata``, and rebuilds a ``fetch_data`` closure
         that resolves the workspace-specific bot token at call time.
 
-        Returns the attachment unchanged when no URL is available.
+        Returns the attachment unchanged when no URL is available.  The
+        URL is re-validated inside the closure (by ``_fetch_slack_file``)
+        rather than here so that a trusted-at-serialize-time URL still
+        fails closed if the allowlist tightens later.
         """
-        meta = attachment.fetch_metadata or {}
-        url = meta.get("url") or attachment.url
+        meta = attachment.fetch_metadata if attachment.fetch_metadata is not None else {}
+        meta_url = meta.get("url")
+        url = meta_url if meta_url is not None else attachment.url
         team_id = meta.get("teamId")
         if not url:
             return attachment

src/chat_sdk/adapters/teams/adapter.py

@@ -17,6 +17,7 @@
 from collections.abc import Awaitable, Callable
 from datetime import datetime, timezone
 from typing import Any, Literal, NoReturn
+from urllib.parse import urlparse
 
 from chat_sdk.adapters.teams.cards import card_to_adaptive_card
 from chat_sdk.adapters.teams.format_converter import TeamsFormatConverter
@@ -591,16 +592,70 @@ def _create_attachment(self, att: dict[str, Any]) -> Attachment:
             fetch_data=self._build_teams_fetch_data(url) if url else None,
         )
 
+    @staticmethod
+    def _is_trusted_teams_download_url(url: str) -> bool:
+        """Gate Teams file downloads to Microsoft-owned hosts.
+
+        After ``rehydrate_attachment`` reconstructs the fetch closure
+        from serialized ``fetch_metadata``, the URL may have been
+        tampered with.  We refuse to issue a direct GET unless the host
+        is a known Microsoft/Graph download host.
+
+        This is a Python-first divergence: upstream Teams adapter does
+        not validate the URL.  See ``docs/UPSTREAM_SYNC.md`` Known
+        Non-Parity.
+        """
+        try:
+            parsed = urlparse(url)
+        except (ValueError, TypeError):
+            return False
+        if parsed.scheme != "https":
+            return False
+        host = (parsed.hostname or "").lower()
+        if not host:
+            return False
+        # Microsoft Graph / Bot Framework / SharePoint / Teams file hosts
+        allowed_suffixes = (
+            ".botframework.com",
+            ".graph.microsoft.com",
+            ".sharepoint.com",
+            ".officeapps.live.com",
+            ".office.com",
+            ".office365.com",
+            ".onedrive.com",
+            ".microsoft.com",
+        )
+        if host.endswith(allowed_suffixes):
+            return True
+        # Exact-match traffic-manager / Graph / Teams service hosts
+        return host in {
+            "smba.trafficmanager.net",
+            "graph.microsoft.com",
+            "attachments.office.net",
+        }
+
     def _build_teams_fetch_data(self, url: str) -> Callable[[], Awaitable[bytes]]:
-        """Build a lazy ``fetch_data`` closure for a Teams file URL."""
+        """Build a lazy ``fetch_data`` closure for a Teams file URL.
 
-        async def fetch_data() -> bytes:
-            import httpx
+        Uses the adapter's shared ``aiohttp.ClientSession`` (via
+        :meth:`_get_http_session`) so downloads reuse the connection
+        pool instead of constructing a throwaway client per request.
+        """
 
-            async with httpx.AsyncClient() as http:
-                resp = await http.get(url)
-                resp.raise_for_status()
-                return resp.content
+        async def fetch_data() -> bytes:
+            if not self._is_trusted_teams_download_url(url):
+                raise ValidationError(
+                    "teams",
+                    f"Refusing to fetch Teams file from untrusted URL: {url}",
+                )
+            session = await self._get_http_session()
+            async with session.get(url) as resp:
+                if resp.status >= 400:
+                    raise NetworkError(
+                        "teams",
+                        f"Failed to fetch file: {resp.status}",
+                    )
+                return await resp.read()
 
         return fetch_data
 
@@ -611,9 +666,12 @@ def rehydrate_attachment(self, attachment: Attachment) -> Attachment:
         need to rebuild the download closure is the URL — either from
         ``fetch_metadata["url"]`` or the attachment's top-level ``url``.
         Returns the attachment unchanged when no URL is available.
+        The URL host is validated inside the closure, so tampered URLs
+        raise at fetch time.
         """
-        meta = attachment.fetch_metadata or {}
-        url = meta.get("url") or attachment.url
+        meta = attachment.fetch_metadata if attachment.fetch_metadata is not None else {}
+        meta_url = meta.get("url")
+        url = meta_url if meta_url is not None else attachment.url
         if not url:
             return attachment
         return Attachment(

src/chat_sdk/adapters/telegram/adapter.py

@@ -1376,7 +1376,7 @@ def rehydrate_attachment(self, attachment: Attachment) -> Attachment:
         no file ID is present (e.g. a pre-serialized attachment that did
         not originate from this adapter).
         """
-        meta = attachment.fetch_metadata or {}
+        meta = attachment.fetch_metadata if attachment.fetch_metadata is not None else {}
         file_id = meta.get("fileId")
         if not file_id:
             return attachment

src/chat_sdk/adapters/whatsapp/adapter.py

@@ -643,7 +643,7 @@ def rehydrate_attachment(self, attachment: Attachment) -> Attachment:
         ``download_media`` closure.  Returns the attachment unchanged when
         no media ID is present.
         """
-        meta = attachment.fetch_metadata or {}
+        meta = attachment.fetch_metadata if attachment.fetch_metadata is not None else {}
         media_id = meta.get("mediaId")
         if not media_id:
             return attachment

src/chat_sdk/chat.py

@@ -2048,13 +2048,21 @@ def _rehydrate_message(self, raw: Any, adapter: Adapter | None = None) -> Messag
         attachment that lost its ``fetch_data`` closure so downstream
         handlers can still download bytes.
         """
-        # Matches upstream: if the entry is already a Message instance, its
-        # fetch_data closures never went through a JSON roundtrip, so we
-        # return it untouched — no rehydrate pass.
+        # Diverges from upstream: upstream TS has
+        # ``if (raw instanceof Message) return raw;`` because its Redis /
+        # Postgres ``dequeue()`` returns the raw ``JSON.parse(value)`` —
+        # never a ``Message`` instance.  Our Python port's Redis +
+        # Postgres ``dequeue()`` already upgrade the raw dict to
+        # ``Message.from_json(...)`` before returning (see
+        # ``state/redis.py`` and ``state/postgres.py``).  An early return
+        # here would therefore skip ``rehydrate_attachment`` for every
+        # dequeued Message in a persistent backend, leaving
+        # ``fetch_data`` stripped.  We fall through and apply the
+        # rehydrate pass; attachments that still have ``fetch_data``
+        # (e.g. in-memory state) are filtered out below.
         if isinstance(raw, Message):
-            return raw
-
-        if isinstance(raw, dict):
+            msg = raw
+        elif isinstance(raw, dict):
             if raw.get("_type") == "chat:Message":
                 msg = _message_from_json(raw)
             else:

src/chat_sdk/types.py

@@ -518,7 +518,9 @@ def from_json(cls, data: dict[str, Any] | Message) -> Message:
                     size=att.get("size"),
                     width=att.get("width"),
                     height=att.get("height"),
-                    fetch_metadata=att.get("fetchMetadata") or att.get("fetch_metadata"),
+                    fetch_metadata=(
+                        att.get("fetchMetadata") if att.get("fetchMetadata") is not None else att.get("fetch_metadata")
+                    ),
                 )
                 for att in attachments_data
             ],
@@ -590,7 +592,9 @@ def from_json_compat(cls, data: dict[str, Any]) -> Message:
                     size=att.get("size"),
                     width=att.get("width"),
                     height=att.get("height"),
-                    fetch_metadata=att.get("fetch_metadata") or att.get("fetchMetadata"),
+                    fetch_metadata=(
+                        att.get("fetch_metadata") if att.get("fetch_metadata") is not None else att.get("fetchMetadata")
+                    ),
                 )
                 for att in attachments_data
             ],