[4.41/MS1] Messenger: guard attachment downloads

[patrick-chinchill]

Summary

The Messenger adapter's Attachment.fetch_data() GETs whatever URL arrived in the webhook payload.url, and it follows redirects. Some Messenger attachment types (fallback, link shares) carry URLs the end user controls, so this is a server-side request forgery (SSRF) weakness. Any handler that calls fetch_data(), directly or through to_ai_messages, can be made to fetch internal or arbitrary hosts. Port upstream hardening 153bd964: fetch only from Meta's CDN hosts, re-check every redirect hop, and cap size and time. This is the minimal, Messenger-local fix. #239 later moves it onto the shared downloader from #204.

Upstream changes

• 153bd964 fix(messenger): guard attachment downloads (#856) — chat@4.39.0 — Messenger downloadAttachment now delegates to a new fetch.ts download(). It permits only fbsbx.com / fbcdn.net (exact host or a subdomain, case-insensitive), requires https, re-validates redirects, uses a 25 MB cap and a 30 s timeout, and wraps failures as NetworkError("messenger", ...). attachment.url stays unchanged, so external URLs are still displayed but never fetched.

Current Python behavior

• src/chat_sdk/adapters/messenger/adapter.py:795-818 _extract_attachments builds fetch_data for any payload.url and stores it in fetch_metadata={"url": url}.
• adapter.py:833-866 rehydrate_attachment rebuilds the closure from fetch_metadata["url"], again with no validation. A tampered queue or debounce entry in state therefore reaches the same code path.
• adapter.py:890-908 _download_attachment calls session.get(url) on the shared session (adapter.py:213-218). It has no scheme or host check, aiohttp's default allow_redirects=True applies, the body is read unbounded (await response.read()), and there is no timeout.
• grep -rn 'fbsbx\|fbcdn' src/chat_sdk/adapters/messenger finds nothing.
• Existing tests use non-Meta hosts and will need new fixtures: tests/test_messenger_webhook.py:940 (test_attachment_has_fetch_data_callable), :954 (test_attachment_download_uses_session, which uses https://example.com/img.jpg), and :1024 (test_rehydrate_rebuilds_fetch_data_after_queue_roundtrip).

Scope

• Add _MESSENGER_MEDIA_HOSTS = ("fbsbx.com", "fbcdn.net") and _is_trusted_messenger_media_url(url) -> bool in messenger/adapter.py. The check requires scheme https, no explicit port (or port 443), and no userinfo. The hostname must equal an allowed host or end with . plus that host, compared case-insensitively. Reject trailing-dot hosts, IP literals and hosts that fail to parse.
• Rewrite _download_attachment(url):
  • validate the URL before any network I/O;
  • run a manual redirect loop (allow_redirects=False, at most 5 hops) that resolves Location against the current URL and re-validates every hop;
  • apply aiohttp.ClientTimeout(total=30);
  • reject a Content-Length above 25 MB, then read in chunks with a running total and abort past 25 MB;
  • treat non-2xx as NetworkError("messenger", f"Failed to fetch file: {status} {reason}");
  • wrap any other exception as NetworkError("messenger", "Failed to download Messenger attachment", original_error=...).
• Error message for a refused URL or hop: "Refusing to fetch an untrusted attachment URL", matching upstream. No network call is made.
• Keep validation inside the download closure, not at parse time, so the rehydrate_attachment path is covered and attachment.url keeps the original value.
• Update the existing Messenger download tests to use https://cdn.fbsbx.com/... URLs.
• Update the Messenger row(s) in docs/UPSTREAM_SYNC.md. Add Messenger to the rehydrate_attachment URL allowlist row (docs/UPSTREAM_SYNC.md:666) and note that upstream now validates too.

Out of scope

• Shared guarded downloader (private-IP / DNS checks, injectable transport): [4.41/SH1] Shared guarded downloader (redirect policy, byte cap, timeout, credential host binding) #204. Migrating Messenger and WhatsApp onto it: [4.41/W4] WhatsApp & Messenger: mark_as_read, native replies, code fences, guarded downloads #239.
• Messenger mark_as_read / mark_seen: [4.41/W4] WhatsApp & Messenger: mark_as_read, native replies, code fences, guarded downloads #239. get_user stays Add MessengerAdapter.get_user (sibling-adapter consistency) #132.
• Raw-body logging in adapters: [4.41/LOG1] Stop logging raw webhook bodies / PII across adapters #187.

Porting notes

• No DNS or private-IP resolution here. The host allowlist alone rejects IP literals and non-Meta names. Connection-bound DNS checks belong to [4.41/SH1] Shared guarded downloader (redirect policy, byte cap, timeout, credential host binding) #204. Record that gap in the sync doc row until [4.41/W4] WhatsApp & Messenger: mark_as_read, native replies, code fences, guarded downloads #239 lands.
• aiohttp follows redirects by default. Pass allow_redirects=False explicitly and handle 301/302/303/307/308 yourself. A missing or malformed Location must raise NetworkError, not KeyError or ValueError.
• Use urllib.parse.urlsplit and .hostname, which lowercases and strips brackets. Compare with == or endswith("." + host), never a bare endswith(host), because suffix attacks like fbcdn.net.attacker.example must fail. Decide explicitly how to treat hostname.endswith(".") (recommended: reject, as upstream does).
• Keep aiohttp lazily imported inside methods (existing pattern at adapter.py:215).
• Everything here is async: the chunked read loop is async for chunk in response.content.iter_chunked(...). Do not introduce blocking reads.
• Do not change the fetch_metadata shape ({"url": ...}), because persisted queue entries depend on it.

Tests

Upstream: packages/adapter-messenger/src/fetch.test.ts (new) and index.test.ts. Neither is fidelity-mapped in scripts/verify_test_fidelity.py MAPPING (adapter tests are not mapped). Port into tests/test_messenger_webhook.py, or a new tests/test_messenger_fetch.py:

• describe("Messenger attachment fetch"):

  • "downloads from Meta CDN URL %s": parametrize over cdn.fbsbx.com, lookaside.fbsbx.com, scontent.xx.fbcdn.net, and the mixed-case SContent.XX.FBCDN.NET.
  • "rejects untrusted attachment URL %s": parametrize over example.com, a suffix-attack host, a trailing-dot host, http://, an IPv4 literal and a decimal-integer host. Assert that the transport is never called.
  • "rejects redirects away from Meta CDN hosts": the transport is called exactly once.
  • "normalizes transport failures"
  • "rejects unsuccessful responses"
  • "uses Messenger network errors"

• index.test.ts: "downloads attachment successfully" (updated to an fbsbx URL) and "rejects external fallback downloads before the network". The second asserts that attachment.url is preserved and the session is not hit.

• Python-specific tests:

  • a redirect between allowed hosts (lookaside.fbsbx.com → scontent.*.fbcdn.net) succeeds, mirroring shared "follows redirects between allowlisted hosts";
  • a Content-Length over the cap is rejected before reading the body;
  • a streamed body over the cap is aborted;
  • a rehydrate_attachment closure built from a non-Meta fetch_metadata["url"] raises NetworkError without I/O.

  Mock the session with an AsyncMock-based context manager, and use no real sleeps. Replace, rather than duplicate, test_attachment_download_uses_session.

Acceptance criteria

• fetch_data() on a non-fbsbx.com/fbcdn.net URL raises NetworkError with zero network calls, both for freshly parsed and for rehydrated attachments.
• Redirects are re-validated per hop, and there are at most 5 of them.
• The 25 MB cap and 30 s total timeout are enforced.
• Full validation command from CLAUDE.md passes (ruff check, ruff format --check, audit_test_quality, verify_test_fidelity, pytest).
• docs/UPSTREAM_SYNC.md is updated (Messenger allowlist row, plus the "no DNS/private-IP check until SH1" note).
• CHANGELOG entry under an "Unreleased (4.41 wave)" heading, marked security.
• Consumer-visible change is called out: Messenger fetch_data() for fallback or link-share attachments on non-Meta hosts now raises instead of downloading.

Dependencies

None — can start immediately. Not an index blocker for anything, but it should land before #239, which replaces this inline guard with the #204 downloader.

Metadata

• Effort: S (<200 LOC)
• Consumer impact: none for Slack/Teams users. Low for Messenger users: external fallback URLs are no longer downloadable, though attachment.url is still present.
• Suggested branch: sync/4.41-ms1

Part of #184.