Skip to content

fix(teams): surface pre-signed content.downloadUrl for chat file attachments #136

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
tony-chinchill-ai wants to merge 1 commit into from
Closed

fix(teams): surface pre-signed content.downloadUrl for chat file attachments #136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## 0.4.29a3 (unreleased)

### Fixes

- **Teams chat file attachments now surface the pre-signed `content.downloadUrl`** — `TeamsAdapter._create_attachment` read only the top-level `contentUrl`, which for a file uploaded in a personal/group chat points at the auth-required SharePoint/OneDrive item and returns 403 to an anonymous GET. The activity also carries `content.downloadUrl`, a short-lived pre-signed link that fetches with no auth header. The adapter now prefers it when present, so the file is readable without granting `Files.Read.All`. Added `tests/test_teams_adapter.py::TestParseMessage::test_prefers_pre_signed_download_url_over_sharepoint_content_url`. Unblocks downstream consumers needing inbound Teams file reads.

### Documentation alignment

- **DM routing precedence docstrings** (vercel/chat#491, commit `67c1794`). The Python runtime already routed direct messages to `on_direct_message` handlers before subscribed-message, mention, and pattern handlers (`Chat._handle_incoming_message`, `chat.py:2154`), but `on_direct_message` and `Thread.subscribe` lacked docstrings clarifying that contract. Refreshed both to match upstream's #491 docstring rewrite, plus a new `tests/integration/test_dm_flow.py::TestDMRoutingDocs` group and a `test_dm_handler_runs_before_pattern_handler` regression test that pin the documented precedence (DM > pattern) the previous suite did not explicitly cover. No runtime behavior change.
Expand Down
8 changes: 7 additions & 1 deletion src/chat_sdk/adapters/teams/adapter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -924,7 +924,13 @@ def _create_attachment(self, att: dict[str, Any]) -> Attachment:
elif content_type.startswith("audio/"):
att_type = "audio"

url = att.get("contentUrl")
# Files uploaded in a personal/group chat arrive as a "download info" attachment
# whose ``content.downloadUrl`` is a short-lived pre-signed link that fetches with
# no auth header. The top-level ``contentUrl`` points at the SharePoint/OneDrive
# item and returns 403 to an anonymous GET, so prefer the pre-signed URL.
content = att.get("content")
download_url = content.get("downloadUrl") if isinstance(content, dict) else None
url = download_url or att.get("contentUrl")

@coderabbitai coderabbitai Bot Jun 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use explicit None check instead of truthiness operator.

Line 933 uses download_url or att.get("contentUrl") which can incorrectly fall back to contentUrl if download_url is an empty string (falsy but not None). As per coding guidelines, use x if x is not None else default to avoid truthiness traps.

🔧 Suggested fix 
-        url = download_url or att.get("contentUrl")
+        url = download_url if download_url is not None else att.get("contentUrl")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
url = download_url or att.get("contentUrl")
url = download_url if download_url is not None else att.get("contentUrl")
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/chat_sdk/adapters/teams/adapter.py` at line 933, The current assignment
url = download_url or att.get("contentUrl") uses truthiness and will incorrectly
fall back when download_url is an empty string; change it to use an explicit
None check so url is download_url if download_url is not None, otherwise
att.get("contentUrl") — update the expression around the url variable where
download_url and att.get("contentUrl") are used (in the Teams adapter code) to
use the form download_url if download_url is not None else
att.get("contentUrl").

Source: Coding guidelines

return Attachment(
type=att_type,
url=url,
Expand Down
28 changes: 28 additions & 0 deletions tests/test_teams_adapter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,34 @@ def test_attachment_stores_url_in_fetch_metadata(self):
assert msg.attachments[0].fetch_metadata == {"url": "https://x.com/photo.jpg"}
assert msg.attachments[0].fetch_data is not None

def test_prefers_pre_signed_download_url_over_sharepoint_content_url(self):
"""A chat file upload arrives as a download-info attachment: ``content.downloadUrl``
is a pre-signed link that fetches anonymously, while the top-level ``contentUrl``
points at the auth-required SharePoint item and 403s. The adapter must surface the
pre-signed URL so the file is actually readable."""
adapter = _make_adapter(app_id="test-app")
signed_url = "https://contoso.sharepoint.com/personal/alice/_layouts/download.aspx?UniqueId=abc&tempauth=SIGNED"
activity = {
"type": "message",
"id": "msg-dl",
"text": "",
"from": {"id": "user-1", "name": "Alice"},
"conversation": {"id": "19:abc@thread.tacv2"},
"serviceUrl": "https://smba.trafficmanager.net/teams/",
"attachments": [
{
"contentType": "application/vnd.microsoft.teams.file.download.info",
"contentUrl": "https://contoso.sharepoint.com/personal/alice/Documents/report.csv",
"name": "report.csv",
"content": {"downloadUrl": signed_url, "uniqueId": "abc", "fileType": "csv"},
},
],
}
msg = adapter.parse_message(activity)
assert msg.attachments[0].url == signed_url, (
"must surface the pre-signed content.downloadUrl, not the auth-required SharePoint contentUrl"
)


# ---------------------------------------------------------------------------
# rehydrate_attachment
Expand Down
Loading