fix(msteams): fix image attachment download for channel and DM messages#40463
fix(msteams): fix image attachment download for channel and DM messages#40463hddevteam wants to merge 5 commits intoopenclaw:mainfrom
Conversation
Greptile SummaryThis PR fixes a real bug where Teams image/file attachments were silently dropped because the Microsoft Graph API never returns Key changes in
Confidence Score: 4/5
Last reviewed commit: 830077f |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 830077f60f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Fixes MSTeams Graph hosted content downloads by handling the Graph API behavior where contentBytes is not inlined in /hostedContents list responses, ensuring Teams images/files aren’t silently dropped.
Changes:
- Decode hosted content from
contentByteswhen present; otherwise fetch bytes via.../hostedContents/{id}/$value. - Capture
content-typefrom the$valueresponse and feed it into MIME detection / storage.
|
The two failing CI checks are pre-existing issues unrelated to this PR:
Both failures reproduce on |
|
Hi @hddevteam — thanks for the submission. I’m the new Microsoft Teams maintainer for OpenClaw. Please give me a day or two to work through the open Teams backlog. Also, join the Twitter community for daily MS Teams feedback + updates: https://x.com/i/communities/2031170403607974228 |
2090dd9 to
eec664e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eec664e0cb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Totally understand, @BradGroux — no rush at all! Thanks for taking on the Teams maintainer role and for letting us know. Looking forward to your review whenever you get a chance. 🙏 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4039a52195
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
4039a52 to
f43d155
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 442621bb7e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
This assigned pull request has been marked as stale after being open for 27 days. |
|
Codex review: needs maintainer review before merge. What this changes: The PR updates Microsoft Teams Graph attachment handling to add channel Maintainer follow-up before merge: This is an active, focused contributor PR already assigned to the Microsoft Teams maintainer; the remaining action is domain validation and normal PR review rather than a separate ClawSweeper repair branch. Security review: Security review cleared: The latest diff is limited to Teams attachment code and tests, adds no dependencies or CI/release changes, uses bounded response reading, and no longer broadens bearer-token forwarding to all Traffic Manager subdomains. Review detailsBest possible solution: Keep the fix inside the Microsoft Teams plugin, land the focused Graph/media hardening after maintainer validation of the Do we have a high-confidence way to reproduce the issue? Yes, for code-level reproduction: current main visibly returns only Is this the best way to solve the issue? Yes, with a domain-validation caveat: the latest branch keeps the behavior in the owning Teams plugin, reuses the existing bounded response helper, and backs out the broad Traffic Manager auth allowlist change. The Acceptance criteria:
What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 9061d1e4c3ee. |
|
This assigned pull request has been automatically marked as stale after being open for 27 days. |
|
This assigned pull request has been automatically marked as stale after being open for 27 days. |
16fbd98 to
7239407
Compare
…inlined
Graph API's /hostedContents collection endpoint does not return contentBytes
inline in the response — the field is always null/absent. The previous code
would skip every item with an empty contentBytes, causing all image/file
attachments sent via Teams to be silently dropped.
Fix: when contentBytes is absent but the item has an id, fall back to fetching
the actual bytes via the individual /{id}/$value endpoint, which always returns
the binary content. The content-type header from that response is also used to
set itemContentType for accurate MIME detection downstream.
Verified working against Microsoft Teams (channel messages) with both image
and file attachments.
…already string | undefined)
…LIST DM image attachments in Microsoft Teams use a direct download URL on smba.trafficmanager.net (the Bot Framework attachment service, SMBA = Service Messaging Bot Attachments). This domain was absent from the auth token allowlist, so download requests were sent without a Bearer token, resulting in 401 responses and silent download failures. Add "trafficmanager.net" so that Bot Framework tokens are automatically attached to requests targeting this domain.
…lback for channels
Two related fixes for buildMSTeamsGraphMessageUrls:
1. DM/group chat: prefer channelData.chatId (the proper Graph chat GUID)
over conversationId, which may be a Bot Framework conversation ID in
the form "a:xxx" that Graph does not recognise.
2. Channel messages: Teams channel IDs surfaced in channelData.team.id
are sometimes in thread format ("19:xxx@thread.tacv2") rather than a
GUID. The /teams/{guid}/channels/ Graph endpoint rejects these with
400. /chats/{threadId}/messages/ accepts thread-format IDs, so append
/chats/ candidate URLs as a fallback after the /teams/ URLs.
7239407 to
e0d695a
Compare
|
This assigned pull request has been automatically marked as stale after being open for 27 days. |
|
This assigned pull request has been automatically marked as stale after being open for 27 days. |
|
Closing due to inactivity. |
Problem
When Teams users send images or file attachments, the bot silently drops them. This PR fixes multiple root causes discovered in a cross-tenant deployment (Bot App registered in Azure subscription tenant A, Teams conversations in M365 org tenant B). All fixes have been validated in production.
Fixes
1. Fetch hostedContents via
/$valuewhencontentBytesnot inlinedThe Graph API
/hostedContentscollection endpoint never returnscontentBytesinline in the list response. The original code treated an emptycontentBytesas "no content" andcontinued, so every attachment was skipped unconditionally.Fix: when
contentBytesis absent but the item has anid, fall back to fetching the binary via/{id}/$value, which reliably returns the file content. TheContent-Typeheader from that response is also captured for accurate MIME detection.2.
some()instead ofevery()for HTML attachment detection (inbound-media.ts)When a channel message contains a mix of image and
text/htmlattachments,every()required ALL attachments to be html before attempting the Graph path. This silently skipped image attachments in mixed messages.Fix: changed to
some()so the Graph path is attempted whenever at least one attachment has atext/htmlcontent type.3. Add
trafficmanager.nettoDEFAULT_MEDIA_AUTH_HOST_ALLOWLIST(shared.ts)DM image attachments use direct download URLs on
smba.trafficmanager.net(SMBA = Service Messaging Bot Attachments, served via Azure Traffic Manager). This domain was missing from the auth token allowlist, so requests were sent without a Bearer token, resulting in 401 and silent download failures.Fix: added
"trafficmanager.net"toDEFAULT_MEDIA_AUTH_HOST_ALLOWLIST.4. Prefer
channelData.chatIdfor DM/group chats; add/chats/fallback for channel messages (graph.ts)Two related fixes in
buildMSTeamsGraphMessageUrls:conversationIdmay be a Bot Framework conversation ID (a:xxx) not accepted by Graph. PreferchannelData.chatId(the proper Graph chat GUID) when available.channelData.team.idis sometimes a Teams thread ID (19:xxx@thread.tacv2) instead of a GUID. The/teams/{guid}/channels/endpoint rejects this with 400./chats/{threadId}/messages/accepts thread-format IDs, so/chats/URLs are now appended as fallback candidates after the/teams/URLs.Testing
@mariozechner/pi-aidependency unrelated to this PR).